Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compiler dependent bug for betr #1362

Merged
merged 3 commits into from
Apr 10, 2017
Merged

Conversation

jinyuntang
Copy link
Contributor

@jinyuntang jinyuntang commented Mar 31, 2017

BeTR failed when running with the 15.x intel compiler. By updating
the compiler configuration and enforce some array shapes during
argument passing fixed the bug.

Fixes #1326
[BFB]

jinyuntang and others added 2 commits March 22, 2017 10:22
With the most recent intel compiler, the betr code crashes when running on more than 24 cpus on edison.
The cause of this crash was traced and found related to the passing of arrays between different subroutines.
Now all arrays are passed with explicit specification of their lower and upper bounds, so betr can run without
noticable issue. However, the advection capability of betr has to be turned off, because the new intel compiler
does not work well with passing generic data types. A workaround is developed in betr-v2, and will be integrated
in the future.

This fix does not affect codes other than betr, so is bfb.
@rljacob
Copy link
Member

rljacob commented Mar 31, 2017

Probably a good bug to fix but we're also trying to move away from Intel 15 to Intel 17.

Copy link
Member

@amametjanov amametjanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like local arrays in called functions were declared with explicit indices and that's why the compiler is complaining about "shape mismatch". So it's a good idea to add indices at call sites to let the compiler check array sizes.

@@ -118,8 +118,9 @@ subroutine betr_tracer_massbalance_check(bounds, lbj, ubj, numf, filter, betrtra
call tracerflux_vars%flux_summary(c, betrtracer_vars)

do kk = 1, ngwmobile_tracers
errtracer(c,kk) = beg_tracer_molarmass(c,kk)-end_tracer_molarmass(c,kk) &
+ tracer_flx_netpro(c,kk)-tracer_flx_netphyloss(c,kk)
if(c>maxval(filter))print*,'crazy happend'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a more informative message here? Maybe print vals of c and maxval(filter).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a left over. I might just delete it.

@@ -1412,7 +1413,6 @@ subroutine diagnose_advect_water_flux(bounds, num_hydrologyc, filter_hydrologyc,

enddo

deallocate(h2osoi_liq_copy)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deallocate was removed, but not added elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This no longer needed.

@@ -1461,7 +1461,6 @@ subroutine diagnose_drainage_water_flux(bounds, num_hydrologyc, filter_hydrology
enddo
enddo

deallocate(h2osoi_liq_copy)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deallocate was removed, but not added elsewhere. May cause a memory leak?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deallocation occurs after the code execution is done. So it follows the implicit deallocation rule as in other extended data types.

@jinyuntang
Copy link
Contributor Author

@rljacob, Yes, I'm aware that we are moving to 17. But that decision to move was made after I explored this bug.

No test needed!

BFB
bishtgautam pushed a commit that referenced this pull request Apr 6, 2017
BeTR failed when running with the 15.x intel compiler. By updating
the compiler configuration and enforce some array shapes during
argument passing fixed the bug.

Fixes #1326
[BFB]
@bishtgautam bishtgautam merged commit b16d114 into master Apr 10, 2017
bishtgautam pushed a commit that referenced this pull request Apr 10, 2017
BeTR failed when running with the 15.x intel compiler. By updating
the compiler configuration and enforce some array shapes during
argument passing fixed the bug.

Fixes #1326
[BFB]
@bishtgautam bishtgautam deleted the jinyuntang/lnd/betr_arrfix1 branch April 18, 2017 21:27
agsalin pushed a commit that referenced this pull request May 1, 2017
Force user to always go through case.submit
jgfouca pushed a commit that referenced this pull request Jun 2, 2017
BeTR failed when running with the 15.x intel compiler. By updating
the compiler configuration and enforce some array shapes during
argument passing fixed the bug.

Fixes #1326
[BFB]
jgfouca pushed a commit that referenced this pull request Feb 27, 2018
BeTR failed when running with the 15.x intel compiler. By updating
the compiler configuration and enforce some array shapes during
argument passing fixed the bug.

Fixes #1326
[BFB]
jgfouca pushed a commit that referenced this pull request Mar 14, 2018
BeTR failed when running with the 15.x intel compiler. By updating
the compiler configuration and enforce some array shapes during
argument passing fixed the bug.

Fixes #1326
[BFB]
rljacob pushed a commit that referenced this pull request Apr 16, 2021
BeTR failed when running with the 15.x intel compiler. By updating
the compiler configuration and enforce some array shapes during
argument passing fixed the bug.

Fixes #1326
[BFB]
rljacob pushed a commit that referenced this pull request May 6, 2021
BeTR failed when running with the 15.x intel compiler. By updating
the compiler configuration and enforce some array shapes during
argument passing fixed the bug.

Fixes #1326
[BFB]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB bug fix PR Land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

array shape alignment bug in betr
4 participants